helm: support gRPC probes and label the service port as gRPC - #587
Draft
wseaton wants to merge 1 commit into
Draft
helm: support gRPC probes and label the service port as gRPC#587wseaton wants to merge 1 commit into
wseaton wants to merge 1 commit into
Conversation
The probe templates only rendered a tcpSocket handler, which an Istio sidecar answers even when the server is down, and the service/container port was named "http", so Istio protocol detection treated the broker's gRPC traffic as HTTP/1.1. Render probe values verbatim (minus "enabled") so grpc/httpGet/exec handlers work, default the probes to the gRPC health service the server already implements, and rename the port to "grpc" with appProtocol set. When an override supplies its own handler the default grpc handler is dropped, since a probe allows only one. Also rename MODEL_EXPRESS_LOGGING_LEVEL to MODEL_EXPRESS_LOG_LEVEL, the env var the server actually reads. Assisted-by: Claude <noreply@anthropic.com> Signed-off-by: Will Eaton <weaton@redhat.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The chart hardcoded tcpSocket probe handlers and named the service/container port
http. Under an Istio sidecar a tcpSocket probe handshakes with the sidecar and reports Ready even when the server is down, and thehttpport name makes Istio protocol detection treat the broker's gRPC as HTTP/1.1, breaking it in-mesh.Changes:
enabled), sogrpc,httpGet,exec, andtcpSockethandlers all work. Defaults switch to the Kubernetesgrpcprobe against the server's gRPC health service, keeping the existing timing values.grpc, withappProtocol: grpcon the Service.MODEL_EXPRESS_LOGGING_LEVELrenamed toMODEL_EXPRESS_LOG_LEVEL, the variable the server actually reads (modelexpress_common/src/envs.rs).Backward compatible: existing values files with
tcpSocketprobes still render. Because Helm deep-merges overrides with the new grpc defaults, the template drops the defaultgrpchandler when an override supplies its own handler, so old-style files do not produce a two-handler probe.Validated with
helm lint helm/andhelm templateagainst defaults,test-values.yaml,values-development.yaml,values-production.yaml,values-local-storage.yaml, plus an old-style tcpSocket override.